Repository navigation
Conversation
spec.template[].helmTemplate.name is passed to "helm template" as the
first positional argument without validation. A value starting with
'-' is parsed by helm as a flag instead of a release name, so arbitrary
helm template flags can be supplied through this field, e.g.
"--set-file=key=<path>" makes helm read a file from the controller (or
sidecarexec) container filesystem into the rendered manifests. The same
value can also be set on a PackageInstall through the
ext.packaging.carvel.dev/helm-template-name annotation.
Return an error before invoking helm when the resolved name or
namespace starts with '-'. Both values come from the App spec (or the
PackageInstall annotations), so they are validated at the point where
the helm arguments are built, which covers both paths. Valid release
names and namespaces can never start with '-', so this does not change
behavior for existing valid configurations.
Changes:
1. pkg/template/helm_template.go
- Reject name and namespace values starting with '-'
2. pkg/template/helm_template_test.go (new)
- Add Test_HelmTemplate_NameAndNamespace covering accepted and
rejected values, and verifying helm is not invoked when rejected
- Add Test_HelmTemplate_NameIsNotParsedAsHelmFlag, which runs helm
with a name of "--set-file=value=<file>" and verifies the file
content is not rendered. Without this change the file content
appears in the helm output.
Signed-off-by: Himanshu Singh <himansh.singh3@gmail.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
spec.template[].helmTemplate.name is passed to "helm template" as the first positional argument without validation. A value starting with '-' is parsed by helm as a flag instead of a release name, so arbitrary helm template flags can be supplied through this field, e.g. "--set-file=key=" makes helm read a file from the controller (or sidecarexec) container filesystem into the rendered manifests. The same value can also be set on a PackageInstall through the ext.packaging.carvel.dev/helm-template-name annotation.
Return an error before invoking helm when the resolved name or namespace starts with '-'. Both values come from the App spec (or the PackageInstall annotations), so they are validated at the point where the helm arguments are built, which covers both paths. Valid release names and namespaces can never start with '-', so this does not change behavior for existing valid configurations.
Changes:
pkg/template/helm_template.go
pkg/template/helm_template_test.go (new)
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?
Additional Notes for your reviewer:
Review Checklist:
a link to that PR
change
Additional documentation e.g., Proposal, usage docs, etc.: